fix(delete): optimize item exist checks when propagating from server
authorMatthieu Gallien <matthieu.gallien@nextcloud.com>
Thu, 15 May 2025 10:28:10 +0000 (12:28 +0200)
committerbackportbot[bot] <backportbot[bot]@users.noreply.github.com>
Fri, 30 May 2025 07:07:46 +0000 (07:07 +0000)
QFileInfo::exists(filename) is the fastest method

alos avoid creating too many QFileInfo instances when we need it for
multiple purposes

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
src/common/filesystembase.cpp
src/libsync/propagatorjobs.cpp

index d2497a3017a544319627482178e54b4653a38936..41919e59f1fffab8805f53f04a46838d53237a02 100644 (file)
@@ -392,8 +392,7 @@ bool FileSystem::fileExists(const QString &filename, const QFileInfo &fileInfo)
     // not valid. There needs to be one initialised here. Otherwise the incoming
     // fileInfo is re-used.
     if (fileInfo.filePath() != filename) {
-        QFileInfo myFI(filename);
-        re = myFI.exists();
+        re = QFileInfo::exists(filename);
     }
     return re;
 }
index 072ccbfd7023b26f19412a0ed7f8e3e0f60b6bbc..c2b929d83f1ed58508919f4a50bd31589022ffe4 100644 (file)
@@ -108,23 +108,26 @@ void PropagateLocalRemove::start()
 
     QString removeError;
     if (_moveToTrash && propagator()->syncOptions()._vfs->mode() != OCC::Vfs::WindowsCfApi) {
-        if ((QDir(filename).exists() || FileSystem::fileExists(filename))
-            && !FileSystem::moveToTrash(filename, &removeError)) {
-            qCWarning(lcPropagateLocalRemove()) << "move to trash failed" << filename << removeError;
-            done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError);
-            return;
+        const auto fileInfo = QFileInfo{filename};
+        if (FileSystem::fileExists(filename, fileInfo)) {
+            if (!FileSystem::moveToTrash(filename, &removeError)) {
+                qCWarning(lcPropagateLocalRemove()) << "move to trash failed" << filename << removeError;
+                done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError);
+                return;
+            }
+        } else {
+            qCWarning(lcPropagateLocalRemove()) << "move to trash failed" << filename << "was already deleted";
         }
     } else {
         if (_item->isDirectory()) {
-            if (QDir(filename).exists() && !removeRecursively(QString())) {
+            if (FileSystem::fileExists(filename) && !removeRecursively(QString())) {
                 done(SyncFileItem::NormalError, tr("Temporary error when removing local item removed from server."), ErrorCategory::GenericError);
                 return;
             }
         } else {
-            if (FileSystem::fileExists(filename)) {
-                const auto fileInfo = QFileInfo{filename};
+            const auto fileInfo = QFileInfo{filename};
+            if (FileSystem::fileExists(filename, fileInfo)) {
                 const auto parentFolderPath = fileInfo.dir().absolutePath();
-
                 const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite};
 
                 if (!FileSystem::remove(filename, &removeError)) {